Skip to content

fix(test_models.py): évite la mutation de classe m.groups#21

Open
miki4iaml wants to merge 2 commits into
CyrilJl:mainfrom
miki4iaml:patch-2
Open

fix(test_models.py): évite la mutation de classe m.groups#21
miki4iaml wants to merge 2 commits into
CyrilJl:mainfrom
miki4iaml:patch-2

Conversation

@miki4iaml

Copy link
Copy Markdown

FIX: l'ancienne version mutait directement les attributs de classe m.groups_ = m.groups_[:2] par affectations au niveau du module.

Bonjour, en recherchant l'origine des fails sur pytest, je relève cette mutation qui pourrait avoir 2 effets indésirables :

  1. La mutation permanente pour toute la durée du processus pytest, affecte tous les tests suivants et tout code qui importait la même classe.
  2. En production, si meteofetch est importé dans un processus long (serveur, notebook), une mutation accidentelle des classes a des effets de bord impossibles à tracer.

La correction proposée crée des sous-classes anonymes locales aux tests, ce qui isole complètement la modification sans toucher aux classes originales.

FIX: l'ancienne version mutait directement les attributs de classe m.groups_ = m.groups_[:2] par affectations au niveau du module. 

Bonjour, en recherchant l'origine des fails sur pytest, je relève cette mutation qui pourrait avoir 2 effets indésirables :
1. La mutation permanente pour toute la durée du processus pytest, affecte tous les tests suivants et tout code qui importait la même classe.
2. En production, si meteofetch est importé dans un processus long (serveur, notebook), une mutation accidentelle des classes a des effets de bord impossibles à tracer.

La correction proposée crée des sous-classes anonymes locales aux tests, ce qui isole complètement la modification sans toucher aux classes originales.
@CyrilJl

CyrilJl commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Je regarde ça demain ou après-demain :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants